Skip to content

Drop ./x suggest #143630

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Drop ./x suggest #143630

wants to merge 5 commits into from
+17 −1,067

Conversation

jieyouxu
Copy link
Member

@jieyouxu jieyouxu commented Jul 8, 2025 •
edited
Loading

This PR removes the current ./x suggest implementation (#109933, #106249) and associated docs for several reasons:

  1. Primarily, ./x suggest is another "flow" in bootstrap that incurs extra complexity and more invariants that bootstrap has to maintain. This causes more friction when trying to investigate and fix staging problems. As far as I know, this flow has not been actively maintained in quite a while, and I'm not aware of interest in maintaining it. Bootstrap really could use less implementation complexity with a very limited maintenance bandwidth.
  2. The current ./x suggest implementation "bypasses" the usual stage defaults for the various check/build/test/etc. flows, and it's not really possible to have a stage default because ./x suggest --run produces a sequence of suggestions like [./x check, ./x test library/std, ..] and then tries to run all of them in sequence, based on which files are modified.
  3. We've not seen a lot of interest both in using it or extending static/dynamic test suggestions. Last extensions were Add x suggest entries for testing mir-opt and coverage #117961 and Suggest pattern tests when modifying exhaustiveness #120763. I'm not convinced the extra implementation complexity is worth it. This was discussed in:

Closes #109933 (the current implementation is being removed).
Closes #143569 (by removing ./x suggest altogether).

@rustbot rustbot added A-meta Area: Issues & PRs about the rust-lang/rust repository itself A-rustc-dev-guide Area: rustc-dev-guide A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jul 8, 2025
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors

This comment was marked as resolved.

Sign in to view
@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 10, 2025
jieyouxu added 5 commits July 12, 2025 15:12
@jieyouxu
Register change tracker warning for removal of ./x suggest
a891b40
@jieyouxu
Remove current implementation of ./x suggest
dc668c1 
This is quite a bit of implementation complexity, yet it is quite
broken, and we don't have the maintenance bandwidth to address.

Remove the current implementation if only to reduce bootstrap's
implementation complexity; the `suggest` flow comes with its own set of
hacks.
@jieyouxu
Regenerate completions after removing ./x suggest
2fdcc81
@jieyouxu
Remove mentions of ./x suggest and suggest-tests in rustc-dev-guide
bd142ce
@jieyouxu
Update triagebot autolabel
9abfa8a
@jieyouxu jieyouxu changed the title [WIP] Drop ./x suggest Drop ./x suggest Jul 12, 2025
@jieyouxu jieyouxu marked this pull request as ready for review July 12, 2025 07:30
@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2025

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 12, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2025

triagebot.toml has been modified, there may have been changes to the review queue.

cc @davidtwco, @wesleywiser

The rustc-dev-guide subtree was changed. If this PR only touches the dev guide consider submitting a PR directly to rust-lang/rustc-dev-guide otherwise thank you for updating the dev guide with your changes.

cc @BoxyUwU, @jieyouxu, @Kobzol, @tshepang

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

tshepang
tshepang reviewed Jul 12, 2025
View reviewed changes
src/bootstrap/src/utils/metrics.rs
@@ -43,7 +43,7 @@ pub(crate) struct BuildMetrics {
state: RefCell<MetricsState>,
}

/// NOTE: this isn't really cloning anything, but `x suggest` doesn't need metrics so this is probably ok.
// NOTE: this isn't really cloning anything, but necessarily for `Build: Clone`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this what is intended

but is necessary for Build: clone

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tshepang tshepang tshepang left review comments

Assignees

@Mark-Simulacrum Mark-Simulacrum

Labels
A-meta Area: Issues & PRs about the rust-lang/rust repository itself A-rustc-dev-guide Area: rustc-dev-guide A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

./x suggest --run produces incorrect test failure Tracking Issue for x suggest
6 participants
@jieyouxu @rust-log-analyzer @bors @rustbot @tshepang @Mark-Simulacrum